Skip to content
This repository has been archived by the owner on Feb 3, 2023. It is now read-only.

Diorama-based app spec tests #1475

Merged
merged 32 commits into from Jun 9, 2019
Merged

Conversation

maackle
Copy link
Member

@maackle maackle commented Jun 3, 2019

PR summary

Based on #1425 and https://github.com/holochain/holochain-rust/tree/reset-conductor-api-after-adding-bridge (PR forthcoming)

Uses diorama instead of nodejs_conductor to run app-spec tests. This is in preparation for removing nodejs_conductor from this repo and ceasing to use it for app-spec tests.

followups

changelog

Please check one of the following, relating to the CHANGELOG-UNRELEASED.md

  • this is a code change that effects some consumer (e.g. zome developers) of holochain core so it is added to the CHANGELOG-UNRELEASED.md (linked above), with the format - summary of change [PR#1234](https://github.com/holochain/holochain-rust/pull/1234)
  • this is not a code change, or doesn't effect anyone outside holochain core development

@maackle maackle changed the base branch from develop to reset-conductor-api-after-adding-bridge June 3, 2019 19:41
Copy link
Member

@zippy zippy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks excellent, pending the failing CI

@@ -6,6 +6,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added
- Error log output added for errors occurring during `hdk::call`, including bridge call errors [#1448](https://github.com/holochain/holochain-rust/pull/1448).
- New `uuid` parameter for `admin/dna/install_from_file`, to set the UUID of the installed DNA, changing its hash
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add in PR ref

};

/// how many milliseconds sleep all bugs under rugs
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol, but also.

Can I get an update/explainer? What's happening that we need to do this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a workaround for #1446

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok cool, well maybe we could put that into the code itself so that there's a good record

fn main() {
if Path::new("../.git/HEAD").exists() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use pathbuf instead? Just handles the seperators better

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't Path just a slice of a PathBuf?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pathbuf is to string as Path is to &str , so what we would want is to PathBuf::().join() so that we dont have to handle the seperator logic for different Oses

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maackle @AshantiMutinta yes and this is my code and i hate it whoops

@Connoropolous
Copy link
Collaborator

@maackle I notice the attempt to do which holochain and which hc

They produce:

./build_and_test.sh: line 6: which: command not found

in the CI. I'm assuming they would do the same locally.

I need to double check to verify which holochain is on the PATH

@maackle
Copy link
Member Author

maackle commented Jun 6, 2019

@Connoropolous

the attempt to do which holochain...

Yeah, @thedavidmeister you think we can add which to nix-shell, or should I take this out? It's a harmless warning but no use keeping it in if it works for nobody (works for me on my host machine though of course)

@lucksus lucksus changed the base branch from reset-conductor-api-after-adding-bridge to develop June 6, 2019 16:59
@lucksus lucksus mentioned this pull request Jun 7, 2019
2 tasks
mkdir -p dist
echo "===================================================================================="
echo "RUNNING cargo test for zomes"
echo "Using conductor binary: `which holochain`"
echo "Using cli binary: `which hc`"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the sources of whichiness @thedavidmeister

@thedavidmeister thedavidmeister merged commit 165d524 into develop Jun 9, 2019
@zippy zippy deleted the diorama-app-spec-tests branch October 4, 2019 18:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants